Add Zstandard compression support and update tests #12201

JakeChampion · 2025-04-18T08:08:38Z

This pull request adds full support for the zstd (Zstandard) compression algorithm throughout Apache Traffic Server, including build system integration, compression plugin support, Accept-Encoding header normalization, and test coverage.

Key Features

Build system and dependencies:

Add CMake support for finding zstd library with new Findzstd.cmake
Update Docker build files to include libzstd-dev package
Add TS_HAS_ZSTD feature flag for conditional compilation

Core compression support:

Extend compress plugin to support zstd compression alongside gzip and brotli
Add zstd stream handling structures and functions
Update compression configuration to include zstd in supported algorithms list

Accept-Encoding header normalization:

Extend proxy.config.http.normalize_ae configuration to support values 4 and 5 for zstd normalization
Update HTTP transaction cache matching to handle zstd encoding

API and infrastructure:

Add TS_HTTP_VALUE_ZSTD and TS_HTTP_LEN_ZSTD constants
Update MIME field handling to recognize zstd encoding
Add zstd support to traffic_layout feature detection

Improved cache matching:

Fix Accept-Encoding quality calculation logic in HTTP transaction cache
Replace multiplicative quality combining with minimum quality selection
Remove problematic gzip-specific fallback logic that caused cache inconsistencies
Ensure proper cache behavior for all compression algorithms

Test Coverage

Comprehensive compress plugin tests covering zstd compression scenarios
Accept-Encoding normalization tests for all zstd combinations
Updated golden files to include zstd compression test results
New compress3.config for zstd-specific plugin configuration
Test all combinations of zstd, br, and gzip in various scenarios
Cache matching tests demonstrate correct behavior without workarounds

Standards Compliance

The implementation follows RFC 8878 standards for zstd compression and maintains backward compatibility with existing gzip and brotli compression functionality. All tests pass and the feature is properly integrated with existing caching and content negotiation mechanisms.

Configuration

New normalization modes:

proxy.config.http.normalize_ae = 4: Prioritize zstd, fallback to br then gzip
proxy.config.http.normalize_ae = 5: Support all combinations of zstd, br, and gzip

Benefits

Improved compression ratios compared to gzip
Better performance than brotli for compression speed
Reduced bandwidth usage and faster page loads
Seamless integration with existing ATS infrastructure

masaori335 · 2025-04-18T23:27:12Z

[approve ci]

JakeChampion · 2025-04-19T14:24:43Z

@masaori335 do you know which files I need to modify to install zstd on the osx used in the Jenkins job?

masaori335 · 2025-04-20T23:09:28Z

We need to ask @ezelkow1 to install the package in the osx env, I guess.

However, this PR has #if HAVE_ZSTD_F check, why it's failing?

JakeChampion · 2025-04-22T07:52:23Z

We need to ask @ezelkow1 to install the package in the osx env, I guess.

However, this PR has #if HAVE_ZSTD_F check, why it's failing?

thank you, _F was a mistake, it was meant to be _H

JakeChampion · 2025-04-22T11:20:27Z

Looks like all the FreeBSD machines for CI are offline

shukitchan

Really appreciate it if you can please add some documentation to the changes.

Thanks.

plugins/compress/compress.cc

cmcfarlen · 2025-04-28T16:51:36Z

Hi! This doesn't compile for me. I added a line to include/tscore/ink_config.cmake.in so HAVE_ZSTD_H would be truthy for the CPP macros. This lit up that code, but then there were several compiler errors:

/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/compress.cc:203:9: error: no member named 'zstd_ctx' in 'Data'; did you mean 'zstd_cctx'?
  203 |   data->zstd_ctx = nullptr;
      |         ^~~~~~~~
      |         zstd_cctx
/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/misc.h:93:14: note: 'zstd_cctx' declared here
   93 |   ZSTD_CCtx *zstd_cctx;
      |              ^
/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/compress.cc:206:11: error: no member named 'zstd_ctx' in 'Data'; did you mean 'zstd_cctx'?
  206 |     data->zstd_ctx = ZSTD_createCCtx();
      |           ^~~~~~~~
      |           zstd_cctx
/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/misc.h:93:14: note: 'zstd_cctx' declared here
   93 |   ZSTD_CCtx *zstd_cctx;
      |              ^
/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/compress.cc:207:16: error: no member named 'zstd_ctx' in 'Data'; did you mean 'zstd_cctx'?
  207 |     if (!data->zstd_ctx) {
      |                ^~~~~~~~
      |                zstd_cctx
/Users/cmcfarlen/projects/oss/trafficserver/plugins/compress/misc.h:93:14: note: 'zstd_cctx' declared here
   93 |   ZSTD_CCtx *zstd_cctx;
      |              ^

Please include this patch in your PR to enable the ZSTD code. Thanks!

diff --git a/include/tscore/ink_config.h.cmake.in b/include/tscore/ink_config.h.cmake.in
index 634ed94c3..7da0771fd 100644
--- a/include/tscore/ink_config.h.cmake.in
+++ b/include/tscore/ink_config.h.cmake.in
@@ -184,3 +184,5 @@ const int DEFAULT_STACKSIZE = @DEFAULT_STACK_SIZE@;
 #cmakedefine YAMLCPP_LIB_VERSION "@YAMLCPP_LIB_VERSION@"

 #cmakedefine01 TS_HAS_CRIPTS
+
+#cmakedefine HAVE_ZSTD_H 1

cmcfarlen

Please patch ink_config.h and resolve compiler issues.

plugins/compress/compress.cc

Comments have been addressed.

# Conflicts: # contrib/docker/ubuntu/noble/Dockerfile # plugins/compress/configuration.cc # plugins/compress/configuration.h

doc/admin-guide/plugins/compress.en.rst

cmcfarlen

Sorry about the terrible delay getting back to this PR!

Autest is failing for me. Please change the 72 back to 7`` for the gzip gold tests.

More generally I don't think the autests will pass on systems without zstd. This is probably ok since it seems at least our autest images do have it.

cmcfarlen · 2025-10-26T00:04:02Z

tests/gold_tests/pluginTest/compress/compress.gold

 < Content-Encoding: gzip
 < Vary: Accept-Encoding
-< Content-Length: 7``
+< Content-Length: 72


This change is causing autest to fail for me on my Mac.

cmcfarlen · 2025-10-27T21:44:38Z

I think the code is fine at this point, but I have a few concerns:

I'm not convinced autests will pass if the library is not available.
There is some code (transact headers, xpack) that unconditionally expect zstd
Maybe we should just require zstd for the build (probably not, and would have to be done for ATS 11)

cmcfarlen · 2025-10-28T22:42:03Z

building without zstd enabled does compile, but the compress autest doesn't run:

% ./autest.sh -f compress
Running Test compress: Skipped
Warning: Skipping test compress because:
 ATS feature not enabled: TS_HAS_ZSTD

Generating Report: --------------
1 Tests were skipped:
--------------------------------------------------------------------------------
 Test "compress" Skipped
    File: compress.test.py
    Directory: /Users/cmcfarlen/projects/oss/trafficserver/tests/gold_tests/pluginTest/compress
  Reason: ATS feature not enabled: TS_HAS_ZSTD

Total of 1 test
  Unknown: 0
  Exception: 0
  Failed: 0
  Warning: 0
  Skipped: 1
  Passed: 0

bryancall · 2025-10-31T19:45:32Z

Comprehensive Testing Results for ZSTD Compression Support

I've completed comprehensive testing of PR #12201 across three test phases. All tests PASSED ✅

Test Environment

Build: dev-asan preset
Test file: 4701 bytes HTML
Configuration: proxy.config.http.normalize_ae: 0 (required for Brotli/ZSTD)

📊 Test Results Summary

Test 1: Baseline (Master Branch)

Established baseline functionality with existing compression algorithms:

✅ GZIP: 4701 → 91 bytes (98.1% reduction)
✅ Brotli: 4701 → 53 bytes (98.9% reduction)

Test 2: PR #12201 WITHOUT ZSTD Libraries

Verified backward compatibility when ZSTD libraries are not available:

✅ GZIP: 4701 → 91 bytes (98.1% reduction)
✅ Brotli: 4701 → 53 bytes (98.9% reduction)
✅ ZSTD: 4701 bytes (uncompressed - graceful fallback)

Result: No regressions. Existing compression algorithms continue to work when ZSTD is unavailable.

Test 3: PR #12201 WITH ZSTD Libraries

Verified new ZSTD compression functionality:

✅ GZIP: 4701 → 91 bytes (98.1% reduction)
✅ Brotli: 4701 → 53 bytes (98.9% reduction)
✅ ZSTD: 4701 → 68 bytes (98.6% reduction) ⭐ NEW!

Result: ZSTD compression works correctly and produces valid Zstandard compressed data.

🎯 Key Findings

✅ ZSTD compression works correctly - Produces valid compressed output with excellent compression ratios
✅ Backward compatibility maintained - PR works with and without ZSTD libraries installed
✅ No regressions - Existing GZIP and Brotli compression continue to work perfectly
✅ Graceful fallback - When ZSTD libraries are unavailable, requests are served uncompressed without errors

📈 Compression Algorithm Performance

On the 4701-byte test file:

Brotli: 53 bytes (best compression)
ZSTD: 68 bytes (+28% vs Brotli)
GZIP: 91 bytes (+72% vs Brotli)

ZSTD achieves excellent compression, sitting between Brotli and GZIP in terms of compression ratio.

⚙️ Configuration Requirements

Critical: For Brotli and ZSTD to function properly, set:

records:
  http:
    normalize_ae: 0

Without this setting, ATS core normalizes the Accept-Encoding header before the compress plugin processes it, which interferes with Brotli/ZSTD compression.

🏗️ Build Notes

The PR's ZSTD detection via find_package(zstd CONFIG) requires a CMake config file. On systems where libzstd-devel doesn't provide zstdConfig.cmake, a simple config file can be created:

add_library(zstd::zstd UNKNOWN IMPORTED)
set_target_properties(zstd::zstd PROPERTIES
  IMPORTED_LOCATION "/usr/lib64/libzstd.so"
  INTERFACE_INCLUDE_DIRECTORIES "/usr/include"
)
set(zstd_FOUND TRUE)

✅ Conclusion

PR #12201 successfully implements ZSTD compression support without any regressions. The implementation:

Maintains full backward compatibility
Handles missing ZSTD libraries gracefully
Achieves excellent compression ratios (98.6% reduction)
Integrates seamlessly with existing compression infrastructure

All three test phases passed successfully. Great work on this feature! 🎉

bryancall · 2025-10-31T20:13:35Z

Code Review - ZSTD Compression Implementation

Thank you for this excellent work on adding ZSTD compression support! The testing shows the feature works well and achieves great compression ratios (98.6%). I've completed a thorough code review and found a few issues that should be addressed before merging.

🔴 Critical Issue

Resource Management in `zstd_transform_init()`

Location: plugins/compress/compress.cc:558-580

Problem: If ZSTD_CCtx_setParameter fails during initialization, the function returns early but leaves data->zstrm_zstd.cctx in a valid (non-NULL) state. The caller at line 395-398 only checks if (!data->zstrm_zstd.cctx), which won't catch these initialization failures. This could lead to compression proceeding with a misconfigured context.

Current Code:

static void
zstd_transform_init(Data *data)
{
  if (!data->zstrm_zstd.cctx) {
    error("Failed to initialize Zstd compression context");
    return;
  }

  size_t result = ZSTD_CCtx_setParameter(data->zstrm_zstd.cctx, ZSTD_c_compressionLevel, ...);
  if (ZSTD_isError(result)) {
    error("Failed to set Zstd compression level: %s", ZSTD_getErrorName(result));
    return;  // ⚠️ Context is valid but misconfigured
  }
  
  result = ZSTD_CCtx_setParameter(data->zstrm_zstd.cctx, ZSTD_c_checksumFlag, 1);
  if (ZSTD_isError(result)) {
    error("Failed to enable Zstd checksum: %s", ZSTD_getErrorName(result));
    return;  // ⚠️ Context exists but checksum not enabled
  }
}

Recommended Fix:

static void
zstd_transform_init(Data *data)
{
  if (!data->zstrm_zstd.cctx) {
    error("Failed to initialize Zstd compression context");
    return;
  }

  size_t result = ZSTD_CCtx_setParameter(data->zstrm_zstd.cctx, ZSTD_c_compressionLevel, data->hc->zstd_compression_level());
  if (ZSTD_isError(result)) {
    error("Failed to set Zstd compression level: %s", ZSTD_getErrorName(result));
    ZSTD_freeCCtx(data->zstrm_zstd.cctx);  // ✅ Clean up
    data->zstrm_zstd.cctx = nullptr;        // ✅ Mark as invalid
    return;
  }
  
  result = ZSTD_CCtx_setParameter(data->zstrm_zstd.cctx, ZSTD_c_checksumFlag, 1);
  if (ZSTD_isError(result)) {
    error("Failed to enable Zstd checksum: %s", ZSTD_getErrorName(result));
    ZSTD_freeCCtx(data->zstrm_zstd.cctx);  // ✅ Clean up
    data->zstrm_zstd.cctx = nullptr;        // ✅ Mark as invalid
    return;
  }

  debug("zstd compression context initialized with level %d", data->hc->zstd_compression_level());
}

⚠️ Medium Issues

1. Integer Overflow in `zstd_compress_operation()`

Location: plugins/compress/compress.cc:514

Problem: Casting int64_t upstream_length to size_t without bounds checking. If upstream_length is negative (malicious input or bug), the cast wraps to a huge positive value.

Current Code:

ZSTD_inBuffer input = {upstream_buffer, static_cast<size_t>(upstream_length), 0};

Recommended Fix:

if (upstream_length < 0) {
  error("Invalid upstream_length: %" PRId64 " (negative value)", upstream_length);
  return false;
}

if (upstream_buffer == nullptr && upstream_length > 0) {
  error("upstream_buffer is NULL with non-zero length");
  return false;
}

ZSTD_inBuffer input = {upstream_buffer, static_cast<size_t>(upstream_length), 0};

2. Similar Issue with `downstream_length`

Location: plugins/compress/compress.cc:520

Problem: Same cast issue, though less critical since it comes from ATS API.

Recommended: Add validation:

if (downstream_length < 0) {
  error("Invalid downstream_length from TSIOBufferBlockWriteStart");
  return false;
}

ℹ️ Minor Issues

1. Typo in Comment

Location: plugins/compress/compress.cc:236

Issue: Comment says "brotlidestory" instead of "brotli destroy"

Fix: Change to // Brotli destroy

2. Inconsistent Error Logging

Location: plugins/compress/compress.cc:395-398

Issue: Mix of TSError() and error() macro.

Current:

if (!data->zstrm_zstd.cctx) {
  TSError("Failed to create Zstandard compression context");  // Uses TSError
  return;
}

Elsewhere:

error("Failed to initialize Zstd compression context");  // Uses error()

Recommendation: Use consistent error logging throughout the ZSTD code - prefer error() macro for consistency with the rest of the file.

✅ Excellent Practices Observed

Proper error checking: All ZSTD errors are checked with ZSTD_isError() and error names are logged
Resource cleanup: ZSTD_freeCCtx() is properly called in data_destroy()
Data integrity: Checksum is enabled with ZSTD_c_checksumFlag
Infinite loop prevention: Code checks for no progress in compression loop (line 542-544)
Proper compression modes: Uses ZSTD_e_continue and ZSTD_e_flush appropriately
Backward compatibility: Gracefully handles missing ZSTD libraries

Summary

Critical Issues: 1 (must fix before merge)
Medium Issues: 2 (should fix for robustness)
Minor Issues: 2 (nice to fix)

Overall Assessment: The ZSTD implementation is well-structured and follows the ZSTD API correctly. The critical initialization issue should be addressed, and the integer overflow checks would significantly improve robustness. Great work on achieving excellent compression ratios and maintaining backward compatibility!

Testing: All three test phases passed successfully ✅

Test 1 (Master baseline): GZIP and Brotli working
Test 2 (PR without ZSTD): No regressions, graceful fallback
Test 3 (PR with ZSTD): ZSTD achieves 98.6% compression (4701 → 68 bytes)

bryancall

I am ok with making the changes in this PR or create another PR, tests worked on my Fedora 42 server.

bryancall · 2025-10-31T19:54:53Z

contrib/docker/ubuntu/noble/Dockerfile

    libbrotli-dev \
+    libzstd-dev \
+    luajit \
+    luajit \


Should only be listed 1 time (luajit)

done in 88a02b2 (#12201)

bryancall · 2025-10-31T20:02:16Z

plugins/compress/configuration.cc

      compression_algorithms_ |= ALGORITHM_DEFLATE;
    } else {
-      error("Unknown compression type. Supported compression-algorithms <br,gzip,deflate>.");
+      error("Unknown compression type. Supported compression-algorithms <zstd,br,gzip,deflate>.");


Looks like this error should be conditional based on how the plugin was compiled

done in 8e4f9c8 (#12201)

bryancall · 2025-10-31T20:07:31Z

plugins/compress/compress.cc

+}
+
+static void
+zstd_transform_init(Data *data)


How do we know if the context is partially initialized? Seems like we should return a bool and check the return value.

done in e03fa5c (#12201)

bryancall · 2025-10-31T20:10:28Z

plugins/compress/compress.cc

+  TSIOBufferBlock downstream_blkp;
+  int64_t         downstream_length;
+
+  ZSTD_inBuffer input = {upstream_buffer, static_cast<size_t>(upstream_length), 0};


No check to see if upstream_length is positive, can cause int overflow issue.

good catch! fixed in b87c864 (#12201)

bryancall · 2025-10-31T20:12:18Z

plugins/compress/compress.cc

+    downstream_blkp         = TSIOBufferStart(data->downstream_buffer);
+    char *downstream_buffer = TSIOBufferBlockWriteStart(downstream_blkp, &downstream_length);
+
+    ZSTD_outBuffer output = {downstream_buffer, static_cast<size_t>(downstream_length), 0};


downstream_length can be an int overflow here too.

completed in 1b1ed1c (#12201)

JakeChampion · 2025-10-31T20:19:28Z

I'll make the changes in this pr if that's okay 👍

Adjusts the condition for suppressing Vary header checks on Accept-Encoding to only apply when explicitly configured by the operator

…atus

JakeChampion force-pushed the zstd branch from 6ffb6ae to 1a113fc Compare April 18, 2025 09:49

JakeChampion marked this pull request as ready for review April 18, 2025 18:34

masaori335 assigned masaori335 and JakeChampion and unassigned masaori335 Apr 18, 2025

masaori335 self-requested a review April 18, 2025 23:25

masaori335 added the compress compress plugin label Apr 18, 2025

masaori335 added this to the 10.2.0 milestone Apr 18, 2025

cmcfarlen self-requested a review April 21, 2025 22:23

JakeChampion force-pushed the zstd branch 2 times, most recently from 0167bf5 to 74cd8b5 Compare April 22, 2025 07:51

JakeChampion force-pushed the zstd branch 4 times, most recently from 308032c to c598282 Compare April 22, 2025 10:12

JakeChampion mentioned this pull request Apr 23, 2025

Add Zstandard compression support to the compress plugin #12211

Draft

shukitchan requested changes Apr 23, 2025

View reviewed changes

JakeChampion commented Apr 23, 2025

View reviewed changes

plugins/compress/compress.cc Outdated Show resolved Hide resolved

bryancall added the New Feature label Apr 23, 2025

bryancall added this to ATS v10.1.x Apr 23, 2025

cmcfarlen requested changes Apr 28, 2025

View reviewed changes

plugins/compress/compress.cc Outdated Show resolved Hide resolved

JakeChampion force-pushed the zstd branch 3 times, most recently from 4b8603e to 72c0dbf Compare May 28, 2025 19:48

JakeChampion requested a review from maskit September 3, 2025 20:38

Merge remote-tracking branch 'upstream/master' into zstd

079b002

# Conflicts: # contrib/docker/ubuntu/noble/Dockerfile # plugins/compress/configuration.cc # plugins/compress/configuration.h

JakeChampion force-pushed the zstd branch from 470880a to 079b002 Compare October 6, 2025 07:43

bneradt requested changes Oct 13, 2025

View reviewed changes

doc/admin-guide/plugins/compress.en.rst Show resolved Hide resolved

JakeChampion requested a review from bneradt October 21, 2025 09:08

JakeChampion added 3 commits October 21, 2025 10:26

Merge remote-tracking branch 'upstream/master' into zstd

c953863

Add flush handling to zstd compression stream

0bb4fb5

refactor zstd compression to reduce code duplication

d31c21b

cmcfarlen reviewed Oct 26, 2025

View reviewed changes

Update compress.gold

3c37f08

bryancall self-requested a review October 27, 2025 21:42

bryancall previously approved these changes Oct 31, 2025

View reviewed changes

bryancall requested a review from cmcfarlen October 31, 2025 20:22

JakeChampion added 8 commits November 3, 2025 10:18

Updates Accept-Encoding variability check logic

7668be8

Adjusts the condition for suppressing Vary header checks on Accept-Encoding to only apply when explicitly configured by the operator

Remove duplicate luajit package from Dockerfile dependencies

88a02b2

only include zstd in error message if zstd exists

8e4f9c8

Merge remote-tracking branch 'upstream/master' into zstd

23716b1

update zstd compression context initialization to return a success st…

e03fa5c

…atus

input validation for zstd compression

b87c864

input validation for zstd compression

1b1ed1c

replace TSError with error for Zstandard compression context failure

6fa64e3

JakeChampion dismissed bryancall’s stale review via 6fa64e3 November 3, 2025 13:38

Merge remote-tracking branch 'upstream/master' into zstd

c1ef84e

JakeChampion requested a review from bryancall November 3, 2025 14:19

Add Zstandard compression support and update tests #12201

Are you sure you want to change the base?

Add Zstandard compression support and update tests #12201

Uh oh!

Conversation

JakeChampion commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Features

Build system and dependencies:

Core compression support:

Accept-Encoding header normalization:

API and infrastructure:

Improved cache matching:

Test Coverage

Standards Compliance

Configuration

Benefits

Uh oh!

masaori335 commented Apr 18, 2025

Uh oh!

JakeChampion commented Apr 19, 2025

Uh oh!

masaori335 commented Apr 20, 2025

Uh oh!

JakeChampion commented Apr 22, 2025

Uh oh!

JakeChampion commented Apr 22, 2025

Uh oh!

shukitchan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cmcfarlen commented Apr 28, 2025

Uh oh!

cmcfarlen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmcfarlen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmcfarlen commented Oct 27, 2025

Uh oh!

cmcfarlen commented Oct 28, 2025

Uh oh!

bryancall commented Oct 31, 2025

Comprehensive Testing Results for ZSTD Compression Support

Test Environment

📊 Test Results Summary

Test 1: Baseline (Master Branch)

Test 2: PR #12201 WITHOUT ZSTD Libraries

Test 3: PR #12201 WITH ZSTD Libraries

🎯 Key Findings

📈 Compression Algorithm Performance

⚙️ Configuration Requirements

🏗️ Build Notes

✅ Conclusion

Uh oh!

bryancall commented Oct 31, 2025

Code Review - ZSTD Compression Implementation

🔴 Critical Issue

Resource Management in zstd_transform_init()

⚠️ Medium Issues

1. Integer Overflow in zstd_compress_operation()

2. Similar Issue with downstream_length

ℹ️ Minor Issues

1. Typo in Comment

2. Inconsistent Error Logging

✅ Excellent Practices Observed

Summary

Uh oh!

bryancall left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

JakeChampion commented Apr 18, 2025 •

edited

Loading

Resource Management in `zstd_transform_init()`

1. Integer Overflow in `zstd_compress_operation()`

2. Similar Issue with `downstream_length`